Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[workflow] Trigger format server when @taichi-gardener is requested for review #1026

Merged
merged 9 commits into from
May 22, 2020
Merged

[workflow] Trigger format server when @taichi-gardener is requested for review #1026

merged 9 commits into from
May 22, 2020

Conversation

archibate
Copy link
Collaborator

@archibate archibate commented May 20, 2020 •
edited
Loading

@archibate archibate changed the title [skip ci] add comment.yml to trigger jobs on comment [workflow] Add comment.yml to trigger jobs on comment May 20, 2020
@archibate
Copy link
Collaborator Author

Hello? Feel free to say No if you don't like this...

@yuanming-hu
Copy link
Member

Hello? Feel free to say No if you don't like this...

Sorry, I've got some work to do over the day, but I'll take a look at this later. Thanks for the PR!

@yuanming-hu
Copy link
Member

My biggest worry about this is that, if a PR author adds a comment for formatting their code, all the reviewers will receive that comment as well. This creates too much noise in the reviewers' mailbox. As a result, they will start to ignore emails from GitHub regarding Taichi.

@archibate
Copy link
Collaborator Author

Then, how about:
Only hook on edited, not created.
So that we can edit the PR description to trigger /format.

@archibate
Copy link
Collaborator Author

I just come up with that, we can hook on review_requested event:
When @taichi-gardener was requested for review, the format server is triggered.

@yuanming-hu
Copy link
Member

I just come up with that, we can hook on review_requested event:
When @taichi-gardener was requested for review, the format server is triggered.

This sounds great!!

@archibate archibate changed the title [workflow] Add comment.yml to trigger jobs on comment [workflow] Trigger format server when @taichi-gardener is requested fo review May 22, 2020
@archibate archibate changed the title [workflow] Trigger format server when @taichi-gardener is requested fo review [workflow] Trigger format server when @taichi-gardener is requested for review May 22, 2020
@archibate archibate removed the request for review from taichi-gardener May 22, 2020 03:14
@archibate
Copy link
Collaborator Author

archibate commented May 22, 2020 •
edited
Loading

OK now!
During @taichi-gardener is on the list of reviewers, all pushes to the PR will trigger a format.
When @taichi-gardener is requested for review, immediately trigger a format at once.

yuanming-hu
yuanming-hu reviewed May 22, 2020
View reviewed changes
Copy link
Member

@yuanming-hu yuanming-hu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! LGTM! Just one minor place that needs discussions.

.github/pull_request_template.md Outdated
Comment on lines 6 to 7

[[Click here for the format server]](http://kun.csail.mit.edu:31415/)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should keep this here for a while, just in case some newcomers do not have access to reviewer assignment?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, sorry, I almost forget that review requesting needs write-access...

@archibate
Copy link
Collaborator Author

We will make persubmit.yml to do the work at some point (after I found a way to write repo in gha).

@archibate
Copy link
Collaborator Author

Hello? I want to merge this for 'enjoyment' in other prs.

yuanming-hu
yuanming-hu approved these changes May 22, 2020
View reviewed changes
Copy link
Member

@yuanming-hu yuanming-hu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thank you! Let's see how it works :-)

@archibate archibate merged commit 8248673 into taichi-dev:master May 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yuanming-hu yuanming-hu yuanming-hu approved these changes

@k-ye k-ye Awaiting requested review from k-ye

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@archibate @yuanming-hu